Skip to content

refactor: Rationalize SQL dialect and statement code, to simplify adding new dialects - #2235

Merged
ferenc-csaky merged 2 commits into
mainfrom
refactor/sql-dialects
Jul 29, 2026
Merged

refactor: Rationalize SQL dialect and statement code, to simplify adding new dialects#2235
ferenc-csaky merged 2 commits into
mainfrom
refactor/sql-dialects

Conversation

@ferenc-csaky

@ferenc-csaky ferenc-csaky commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Key Changes

  • Centralized dialect SQL translation discovery and dispatch.
  • Made translation maps immutable and locale-safe.
  • Consolidated relational-plan and SQL-string conversion into SqlConverters.
  • Replaced duplicated dialect converter pairs with shared conversion logic.
  • Added service-based lookup for dialect SQL converters.
  • Updated JDBC statement factories to use the consolidated converter.

Snapshot Diffs

  1. I turned back on identifier quoting in Snowflake
  2. Some postgres ocnverters were instantiated multiple times before, that causes the generated index diff

@ferenc-csaky
ferenc-csaky force-pushed the refactor/sql-dialects branch from dfff128 to e589ac2 Compare July 27, 2026 13:28
@codecov

codecov Bot commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 31.08108% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.10%. Comparing base (98a02a8) to head (e589ac2).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...sqrl/calcite/dialect/SqlTranslationDispatcher.java 0.00% 17 Missing ⚠️
...atasqrl/calcite/convert/AbstractSqlConverters.java 7.14% 13 Missing ⚠️
...abase/relational/AbstractJdbcStatementFactory.java 0.00% 8 Missing ⚠️
...tasqrl/calcite/convert/SnowflakeSqlConverters.java 50.00% 2 Missing ⚠️
...tasqrl/calcite/dialect/BasePostgresSqlDialect.java 0.00% 2 Missing ⚠️
...database/relational/SnowflakeStatementFactory.java 0.00% 2 Missing ⚠️
.../java/com/datasqrl/calcite/SqrlConfigurations.java 0.00% 1 Missing ⚠️
...m/datasqrl/calcite/convert/FlinkSqlConverters.java 50.00% 1 Missing ⚠️
...com/datasqrl/calcite/dialect/DuckDbSqlDialect.java 83.33% 1 Missing ⚠️
...rl/calcite/dialect/ExtendedPostgresSqlDialect.java 87.50% 1 Missing ⚠️
... and 3 more
Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #2235      +/-   ##
============================================
+ Coverage     16.91%   17.10%   +0.18%     
- Complexity     1040     1045       +5     
============================================
  Files           613      611       -2     
  Lines         17738    17693      -45     
  Branches       2155     2155              
============================================
+ Hits           3001     3026      +25     
+ Misses        14425    14355      -70     
  Partials        312      312

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

mbroecheler
mbroecheler approved these changes Jul 27, 2026
View reviewed changes

@mbroecheler mbroecheler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I did not validate the snapshot changes.

@ferenc-csaky ferenc-csaky added this to the 0.11.0 milestone Jul 29, 2026
@ferenc-csaky ferenc-csaky added the enhancement New feature or request label Jul 29, 2026
@ferenc-csaky
ferenc-csaky merged commit 6ec85e5 into main Jul 29, 2026
20 of 21 checks passed
@ferenc-csaky
ferenc-csaky deleted the refactor/sql-dialects branch July 29, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbroecheler mbroecheler mbroecheler approved these changes
@mateczagany mateczagany mateczagany approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

0.11.0

Development

Successfully merging this pull request may close these issues.

3 participants

@ferenc-csaky @mbroecheler @mateczagany